Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

android: make system fonts toggable #5670

Merged
merged 2 commits into from
Dec 8, 2019

Conversation

pazos
Copy link
Member

@pazos pazos commented Dec 6, 2019

like on desktop linux, but without the menu entry to open fonts dir.

Wip: works fine on fresh installation (without the koreader folder or with the folder without further modifications) but crashes on my main device, which is actually is/was heavily modified.

I'll update the crash logs when I have a bit of time.

Edit: the main reason for this PR is to avoid 15 pages of noto fonts on my huawei tablet. Others will find this useful too, specially on generic, modern, phones/tablets.


This change is Reviewable

@NiLuJe
Copy link
Member

NiLuJe commented Dec 6, 2019

Oh, yeah, +1, I also have a bazillion Noto variants on my OnePlus phone ;).

@pazos
Copy link
Member Author

pazos commented Dec 7, 2019

okey, the error happens if/when a system font was used for a document and now is disabled:

12-07 11:32:44.131 12016 12036 E KOReader: [NativeThread]: failed to run script: frontend/ui/widget/textwidget.lua:68: attempt to index field 'face' (a nil value)
12-07 11:32:44.131 12016 12036 E KOReader: [NativeThread]: Stopping due to previous errors

It should affect desktop linux too, but I didn't test.

@poire-z
Copy link
Contributor

poire-z commented Dec 7, 2019

a system font was used for a document
textwidget.lua:68: attempt to index field 'face' (a nil value)

That's not for a document, but for text in the UI elements - which should use only the NotoSans & Free* fonts we define in font.lua - which we still ship on Android.
So, the question is whether disabling system fonts make KOReader not use its own shipped fonts - or if, in spite of this disabling, it would still look for them, see them before the shipped ones, and ignore the shipped ones, and end up not using them because they are disabled (haven't look at the code, just some naive thoughts).

(Just checked how crengine behave when the main and the fallback fonts previously set disappear: it does not crash, and seem to use some Noto font.)

@pazos
Copy link
Member Author

pazos commented Dec 7, 2019

@poire-z: thanks for the info. I was stupid enough and didn't notice the error. This time with a debug build. As you can see the error is not related to noto, but with DroidSansFallback:

12-07 15:34:28.887 14143 14163 V KOReader:  Found font: NotoSans-Regular.ttf in ./fonts/noto/NotoSans-Regular.ttf
12-07 15:34:28.899 14143 14163 V KOReader:  Found font: NotoSans-Regular.ttf in ./fonts/noto/NotoSans-Regular.ttf
12-07 15:34:28.899 14143 14163 V KOReader:  Found font: DroidSansMono.ttf in ./fonts/droid/DroidSansMono.ttf
12-07 15:34:28.901 14143 14163 V KOReader:  Found font: NotoSansCJKsc-Regular.otf in ./fonts/noto/NotoSansCJKsc-Regular.otf
12-07 15:34:28.905 14143 14163 E KOReader:  #! Font  DroidSansFallback.ttf  ( DroidSansFallback.ttf ) not supported:  ffi/freetype.lua:142: Failed to load font './fonts/DroidSansFallback.ttf', freetype error code: 1

Reverting 503a57b fixes the error

@pazos pazos requested a review from Frenzie as a code owner December 7, 2019 15:03
@pazos pazos changed the title [wip] android: make system fonts toggable android: make system fonts toggable Dec 7, 2019
@pazos pazos added this to the 2019.12 milestone Dec 7, 2019
@poire-z
Copy link
Contributor

poire-z commented Dec 7, 2019

The alternative to ressurecting all the fonts we got rid of :) would be to still pick "our no more shipped" fonts from system fonts, even when system fonts are disabled - which might be a bit tedious to code :|
But as we ressurected all of them except that DroidSans (did we?), I guess it's fine just getting it back and forget about that tricky font picking stuff.

@Frenzie
Copy link
Member

Frenzie commented Dec 7, 2019

Alright, let's just revert it. :-/ The Droid Sans one is pretty small anyway; it's big fat Noto I wanted to get rid of.

@poire-z poire-z merged commit a5069f1 into koreader:master Dec 8, 2019
mwoz123 pushed a commit to mwoz123/koreader that referenced this pull request Mar 29, 2020
Also add droid font back to android and remove the workaround
@pazos pazos deleted the android_system_fonts branch December 11, 2020 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants